Skip to content

Reconcile badge / issue count after a backup import#88

Merged
kyleve merged 3 commits into
mainfrom
cursor/backup-import-badge-refresh
Jul 15, 2026
Merged

Reconcile badge / issue count after a backup import#88
kyleve merged 3 commits into
mainfrom
cursor/backup-import-badge-refresh

Conversation

@kyleve

@kyleve kyleve commented Jul 15, 2026

Copy link
Copy Markdown
Owner

Problem

After exporting and then importing data (replace all), the app-icon (home-screen) badge stayed at its stale pre-import value (e.g. 157) — the badge / issue count was never re-run.

BackupCoordinator.importBackup committed the import inside one store.perform and then only republished the widget snapshot:

try await store.perform { /* clearAll (replace) + re-add rows */ }
await widgets.publish()

The iOS app-icon badge is set exclusively by ReminderReconciler.reconcile() (missingDayBacklog.count + unresolvedIssueCount), and the "issues to resolve" notification by DataIssueAlertReconciler.reconcile(). Neither observes store.changes(), so an import left both stuck at their pre-import values until an unrelated reconcile trigger (next foreground, a GPS ingest, or a DayJournal write). Every other bulk data change — e.g. DayJournal.eraseAllData() — already runs the full post-write reconcile fan-out; the import path did not.

(The in-app Resolve tab badge, YearReportModel.dataIssueCount, already refreshed correctly off the import's store.changes() ping — this was specifically the headless app-icon badge and issue-alert notification.)

Fix

An import changes day data, so it needs the same post-day-change reconcile a journal write runs. Rather than reach into all those reconcilers from the backup path (a leaky abstraction, and a duplicated fan-out), the coordinator takes a single hook the composition root supplies:

  • BackupCoordinator now takes onImport: @Sendable () async -> Void and invokes it once after an import commits — nothing else. It depends only on the store + the hook (the scanner/reminder/issue-alert/widget dependencies are gone).
  • WhereServices wires the hook to the existing fan-out: onImport: { await journal.reconcileAfterDayChange() }. DayJournal.reconcileAfterDayChange() (scanner invalidate → reminders/issueAlerts reconcile → widget publish) is now internal so it's reused, keeping the reconcile defined in exactly one place.

Tests

  • BackupCoordinatorTests.replaceImportInvokesTheOnImportHook — the coordinator invokes its onImport hook exactly once after an import (and the merge round-trip asserts the same).
  • WhereServicesTests.backupReplaceImportRefreshesTheAppIconBadge — end-to-end guard through the assembled services: a replace import recomputes the app-icon badge off the imported data (drops from 5 to 0), exercising the real wired hook.

Validation

  • ./swiftformat --lint is clean.
  • tuist test WhereCoreTests passes on iPhone 17 / iOS 26.2 (including the tests above).
Open in Web Open in Cursor 

kyleve added 2 commits July 15, 2026 13:20
BackupCoordinator.importBackup only republished the widget snapshot after
committing, so the app-icon (home-screen) badge and the issues-to-resolve
notification kept their pre-import values until an unrelated reconcile
trigger. Wire the scanner + reminder + issue-alert reconcilers into
BackupCoordinator and, after an import commits, run the same post-write
fan-out DayJournal uses (scanner invalidate -> reminders/issueAlerts
reconcile -> widget publish).

Plan steps: wire-coordinator, wire-services.
Add a BackupCoordinator regression test and a WhereServices end-to-end
test proving a replace import recomputes the app-icon badge off the
imported data instead of leaving the stale pre-import count.

Plan steps: test-coordinator, test-e2e.
private var previousExportDirectory: URL?

init(store: any WhereStore, widgets: WidgetSnapshotPublisher) {
init(

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I know I approved this plan but this seems like a leaky abstraction to me. What if instead of injecting all of these, we just had this take in an onImport closure that our builder would invoke these rebuilds and updates in?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Eg, that would let us DRY up this into one place and not duplicate it.)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Posted by an AI agent on @kyleve's behalf.

Good call — done in 6fab57d. BackupCoordinator no longer takes the scanner/reminder/issue-alert reconcilers (or the widget publisher); it just takes a single onImport: @Sendable () async -> Void and calls it once after an import commits. The composition root wires that hook to the existing fan-out:

let backup = BackupCoordinator(
    store: store,
    onImport: { await journal.reconcileAfterDayChange() },
)

DayJournal.reconcileAfterDayChange() went from private to internal so it's reused rather than duplicated — an import now runs the exact same post-day-change reconcile a journal write does, defined in one place. The coordinator test now just asserts the hook fires once per import; the end-to-end badge recount stays covered by WhereServicesTests.

Address review: injecting the scanner + reminder + issue-alert reconcilers
into BackupCoordinator was a leaky abstraction and duplicated DayJournal's
post-day-change fan-out. Instead the coordinator takes a single onImport
closure and the composition root points it at journal.reconcileAfterDayChange(),
so the invalidate/reconcile/publish fan-out stays defined in one place. The
coordinator now depends only on the store + the hook.
@kyleve kyleve marked this pull request as ready for review July 15, 2026 21:14
@kyleve kyleve enabled auto-merge (squash) July 15, 2026 21:15
@kyleve kyleve merged commit 1456870 into main Jul 15, 2026
2 checks passed
kyleve added a commit that referenced this pull request Jul 15, 2026
## What

Trims the largest `AGENTS.md` files down to invariants, hard-fought lessons, and instructions — dropping structure enumerations agents re-derive from the manifests in their first minute of searching — and regroups WhereCore's flat `Sources/` into concern-based subdirectories. One commit per step.

**Guiding principle:** an AGENTS.md keeps invariants, lessons, conventions, and recipes; it drops module lists, directory trees, and dependency catalogs that mirror `Package.swift` / `Project.swift`. The root file's libraries list was already stale (missing `WhereIntents` and `SwiftDataInspector`) — proof that content rots.

## Commits

1. **GitHub rules** — new root **GitHub** section: use `gh` for all GitHub interaction; open PRs ready-for-review, never draft; keep open PRs current by pushing each local commit. "Working on PR feedback" now requires replying to a comment when a commit resolves it, and filing deferred feedback durably instead of dropping it.
2. **Root trim** — libraries/targets enumerations become pointers at the manifests; directory layout collapses to the module skeleton. The add-a-target recipe, CI-scheme rule, and the double-linking lesson stay verbatim.
3. **Where trim** — Modules tree becomes a one-paragraph layering stack; developer-overlay tour and feature narration cut; the stylesheet paragraph now points at `WhereUI/AGENTS.md`. 222 → 189 lines.
4. **Module trims** — WhereUI / WhereCore / WhereIntents lose their `Package.swift`-mirroring dependency listings; PeriscopeCore loses its directory catalog. All invariants and the full `WhereStylesheet` guidance stay.
5. **WhereCore regroup** — pure `git mv`, no behavior change: stragglers fold into `Backup/`, `Reminders/`, `Widgets/`, `Location/`, `Evidence/`, `DataResolution/`, `Persistence/`; new `Days/`, `Reporting/`, `Journal/`, `Preferences/` groups. Only `WhereServices*` and `WhereLog` remain top-level. Tests stay flat, matching PeriscopeCore.

## New lessons codified (mined from the last 30 days of PR review)

- Prefer synthesized `Codable`; persisted wire formats must survive Swift renames; hand-written conformances document their load-bearing reason (#85, #86)
- Retain block-based notification-observer tokens; every `start` has a paired `stop()` (#69)
- Group large flat types into sub-structs / child types per behavioral area (#47, #69)
- Test doubles conform to the production protocol, never an enum switch in a production type (#54)
- Custom full-screen surfaces are accessibility-modal and post focus notifications (#70)
- Derive UI dimensions from the live UI (preference keys, `@ScaledMetric`, semantic font styles) instead of repeating magic numbers (#38, #70, #83)
- WhereCore: post-write reconciliation is defined once — writes/imports route through `DayJournal.reconcileAfterDayChange()`; cross-collaborator hooks are a single closure (#80, #88)

## Verification

- `./swiftformat --lint` clean; `./sync-agents` run after each docs commit
- Full `Stuff-iOS-Tests` scheme after the regroup: 1183 tests, 0 failures

Made with [Cursor](https://cursor.com)
kyleve added a commit that referenced this pull request Jul 15, 2026
…store-backed tracked regions (#87)

## What & why

Prepares RegionKit for "more locations" ahead of letting users pick their own regions. Two coupled shifts:

1. **Split** the ~2.5 MB monolithic `us-states.geojson` into one GeoJSON file per region, loaded **on demand** — so we only ever parse the regions we track, not the whole US at launch.
2. **Replace** the hardcoded five-case `Region` enum with a **data-driven catalog** (a bundled `regions.json` manifest + a `Region` value type), so any US state (50 + DC + PR) plus Canada/EU are available regions. The user's **tracked** regions live in the synced SwiftData store (WhereCore owns "which"; RegionKit stays the agnostic geometry/lookup engine).

Onboarding's region picker and user-chosen per-region styling are **out of scope** here (designed for, not built).

## What changed

- **RegionKit — data-driven catalog + on-demand geometry**
  - `Region` is now a `Hashable`/`Codable` value type over a stable id (`us-CA`, `canada`, …) with a well-known `.other` and conveniences (`.california`, etc.). `RegionCatalog` loads the bundled `regions.json` manifest (all/name/localizedName/geometry file, canonical order). Adding a region is now **pure data** — a manifest + geojson change generated by `Tools/generate-regions.rb`, no code.
  - `RegionAttributor(for:)` loads only the passed regions' files; `.all` covers the whole catalog, `.shared` the default four. `RegionAttributing` abstracts the engine so the app can supply a live, swappable attributor. `RegionGeometryCatalog.outlines(for:attributor:)` takes an explicit attributor.
- **WhereCore — store-backed, synced tracked regions + live attributor**
  - Tracked regions persist as **one `SDTrackedRegion` row per region** (so concurrent cross-device edits merge instead of last-write-wins), read as a `Set` defaulting to the four until the user chooses.
  - `RegionAttribution` rebuilds the attributor from the tracked set on the store's `changes()` signal (local edit or remote CloudKit import). `WhereServices.make(...)` (async) derives it from the store; the app launch and the App Intents process (`WhereServices.forIntents()`, now async) both attribute against the same synced set. `make(...)` is the **sole public assembly entry**; the synchronous `init` is `@_spi(Testing)` for tests/previews.
- **WhereUI / WhereIntents — catalog-driven**
  - `RegionStyle` is id-keyed with an isolated, disposable bespoke-override table (the seam user-chosen styling will replace). Ordering uses `Region.inCanonicalOrder(_:)` instead of scanning `Region.allCases`.
  - Siri's "pick a region" suggestions and the Spotlight index surface the **tracked** set; `RegionEntity` id resolution stays **full-catalog** (so "days in Texas" answers even when untracked).
- **Backup** — tracked regions round-trip in the archive (additive field; v1 manifests decode to `[]`); `.replace` restores the set exactly, `.merge` unions it.

## Key decisions / tradeoffs

- **Clean id scheme** (`us-CA` vs `california`): existing persisted region raw values change; data is re-exported/re-imported. The `NAME → id` map lives in `Tools/generate-regions.rb`.
- **Localization tradeoff:** region names come from the manifest (with an optional `localizationKey` override), so region names lose static string-catalog extraction — consistent with how the App Intents `RegionEntity` already resolves names at runtime.
- **Attribution priority** is the manifest's canonical order (regions are mutually exclusive at our resolution).

## Deferred (follow-ups)

- The onboarding region picker and user-chosen per-region color/emoji/symbol/tint.
- Seeding semantics (the default four apply only at zero rows; the picker will seed/replace) and materializing the implicit default before the first explicit edit.
- **Untracking a region:** the store currently *deletes* the tracked-region row, which would re-attribute that region's past GPS days to `.other` on re-aggregation (manual days, stored as region sets, are safe). A `TODO` defers the soft-delete (mark inactive + load every ever-tracked region so history stays stable) to the picker work that defines the untrack UX. Not user-reachable today — nothing calls `setTrackedRegion(false)` outside tests.

## Testing

- Full `Stuff-iOS-Tests` scheme green; `./swiftformat --lint` clean.
- New/reworked coverage: catalog + value-type (`RegionTests`), subset-only-loads (`RegionAttributorTests`), rebuild-on-`changes()` + store round-trip (`RegionAttributionTests`, `TrackedRegionStoreTests`), end-to-end `make()` wiring (`WhereServicesTests`), tracked-set entity/suggestions (`RegionEntityTests`), and backup round-trip / legacy-manifest decode (`BackupServiceTests`, `BackupCoordinatorTests`).

## Review pass

- **Self-review:** log (not swallow) tracked-region read failures; deterministic (canonical-ordered) attributor build; surface unknown stored region ids; backup round-trip (above).
- **PR review feedback:** consolidated the RegionKit bundled-data docs into the main `README.md` (removed the separate `Resources/README.md`); documented why `Region`'s `Codable` is hand-written (bare id string vs the synthesized keyed object); made the sync `WhereServices.init` `@_spi(Testing)` so `make()` is the sole public entry; and TODO'd the untrack soft-delete (see Deferred).

## Kept current with `main`

Merged `#85` (CalendarDay), `#86` (Periscope JournalKit), `#88` (backup `onImport` reconcile hook), and `#89` (streamlined AGENTS + WhereCore source regroup). Conflicts resolved: the backup `formatVersion` (kept v2 **and** the additive `trackedRegions` field); the `BackupCoordinatorTests` harness swap (kept both the tracked-region tests and `#88`'s `onImport` hook test); and `Where/AGENTS.md` (took `#89`'s streamlined Modules section, but kept the accurate manifest-backed RegionKit localization note over `#89`'s stale "static keys" wording). My changes rode the source regroup into `Backup/`, `Persistence/`, `Days/`, `DataResolution/`, etc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant